Skip to content

Migrate from doctrine/annotations to PHP 8 Attributes#165

Merged
koriym merged 20 commits into
1.xfrom
php82
Nov 10, 2025
Merged

Migrate from doctrine/annotations to PHP 8 Attributes#165
koriym merged 20 commits into
1.xfrom
php82

Conversation

@koriym
Copy link
Copy Markdown
Member

@koriym koriym commented Nov 8, 2025

Summary

This PR removes the abandoned doctrine/annotations dependency and migrates to native PHP 8 Attributes. The migration includes comprehensive tooling and documentation to help users upgrade their applications.

Changes

Core Migration

  • Remove doctrine/annotations and doctrine/cache dependencies from composer.json
  • Migrate HttpCacheInterceptor to use PHP 8 Attributes reflection API
  • Migrate QueryRepository to use PHP 8 Attributes reflection API
  • Remove Reader dependency from QueryRepository constructor

User Migration Support

  • Add rector-migrate.php for automated annotation-to-attribute conversion
  • Add ANNOTATION_TO_ATTRIBUTE.md with comprehensive migration guide including:
    • Why migration is necessary (doctrine/annotations is abandoned)
    • Step-by-step migration instructions
    • Before/after code examples for all annotations
    • Troubleshooting guide
    • Manual migration instructions

Documentation

  • Update CHANGELOG.md with migration information and new release date (2025-11-09)
  • Add migration tools to Added section
  • Document removal of doctrine dependencies

Affected Annotations

Users need to migrate from docblock annotations to attributes:

  • @Cacheable#[Cacheable]
  • @HttpCache#[HttpCache]
  • @NoHttpCache#[NoHttpCache]
  • @Refresh#[Refresh]
  • @Purge#[Purge]
  • @DonutCache#[DonutCache]

Migration Path

Users can migrate automatically using the provided Rector configuration:

vendor/bin/rector process src --config=vendor/bear/query-repository/rector-migrate.php

See ANNOTATION_TO_ATTRIBUTE.md for detailed instructions.

Why This Change?

The doctrine/annotations package has been officially abandoned by its maintainers and will no longer receive security updates. This change:

  • Eliminates security and compatibility risks
  • Adopts native PHP 8 attributes as the modern standard
  • Improves performance (no runtime annotation parsing)
  • Provides automated migration tooling for users

Test Plan

  • All existing tests pass
  • Code migrated to use Reflection API for attributes
  • Migration guide tested and validated
  • Rector configuration tested

🤖 Generated with Claude Code

Summary by Sourcery

Migrate off doctrine/annotations to native PHP 8 attributes by removing abandoned dependencies, refactoring core classes and tests to use the Reflection API for attributes, and providing automated migration tooling and documentation.

New Features:

  • Add rector-migrate.php configuration for automated annotation-to-attribute conversion

Enhancements:

  • Remove doctrine/annotations and doctrine/cache dependencies and migrate HttpCacheInterceptor and QueryRepository to use PHP 8 attributes via Reflection API
  • Drop the Reader dependency from QueryRepository and update core logic to instantiate attributes directly

Build:

  • Remove doctrine/annotations and doctrine/cache entries from composer.json

Documentation:

  • Add ANNOTATION_TO_ATTRIBUTE.md with a comprehensive migration guide
  • Update CHANGELOG.md with migration details and new release date (2025-11-09)

Tests:

  • Remove AnnotationReader usage in tests following migration to attributes

koriym and others added 2 commits November 6, 2025 18:24
Remove doctrine/annotations and doctrine/cache dependencies from composer.json and replace annotation reader usage with native PHP 8 Attributes API.

Changes:
- QueryRepository: Replace Reader::getClassAnnotation() with ReflectionClass::getAttributes()
- HttpCacheInterceptor: Replace getAnnotation() with getAttributes() for HttpCache and NoHttpCache attributes
- Remove Reader dependency from QueryRepository constructor
- Update test code to match new constructor signature

All tests pass successfully (103 tests, 200 assertions).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Provide tooling and documentation to help users migrate from Doctrine annotations to PHP 8 attributes:

- Add rector-migrate.php: Rector configuration for automated migration of BEAR.QueryRepository annotations
- Add ANNOTATION_TO_ATTRIBUTE.md: Comprehensive migration guide with:
  - Explanation of why migration is necessary (doctrine/annotations is abandoned)
  - Step-by-step migration instructions
  - Before/after code examples
  - Troubleshooting guide
  - List of all supported annotations
- Update CHANGELOG.md: Update release date to 2025-11-09 and add migration tool entries

This supports users in migrating their applications away from the abandoned doctrine/annotations package to native PHP 8 attributes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Nov 8, 2025

Reviewer's Guide

This PR replaces Doctrine annotations with native PHP 8 attributes by removing the abandoned doctrine/annotations and doctrine/cache dependencies, refactoring the interceptor and repository classes to use the ReflectionAttribute API, updating tests accordingly, and shipping migration tooling and documentation to guide users through the upgrade.

Entity relationship diagram for migrated annotation attributes

erDiagram
    RESOURCE_OBJECT {
        string id
        string name
    }
    CACHEABLE {
        string expiry
        string type
        int expirySecond
        bool update
    }
    HTTP_CACHE {
        int maxAge
        int sMaxAge
        bool isPrivate
        bool mustRevalidate
        array etag
    }
    REFRESH {
        string uri
    }
    PURGE {
        string uri
    }
    DONUTCACHE {
    }
    RESOURCE_OBJECT ||--o| CACHEABLE : "can have"
    RESOURCE_OBJECT ||--o| HTTP_CACHE : "can have"
    RESOURCE_OBJECT ||--o| REFRESH : "can have"
    RESOURCE_OBJECT ||--o| PURGE : "can have"
    RESOURCE_OBJECT ||--o| DONUTCACHE : "can have"
Loading

Class diagram for updated HttpCacheInterceptor and QueryRepository

classDiagram
    class HttpCacheInterceptor {
        +invoke(MethodInvocation $invocation)
    }
    class QueryRepository {
        -RepositoryLoggerInterface $logger
        -HeaderSetter $headerSetter
        -ResourceStorageInterface $storage
        -Expiry $expiry
        +purge(AbstractUri $uri)
        +getHttpCacheAnnotation(ResourceObject $ro): HttpCache|null
        +getCacheableAnnotation(ResourceObject $ro): Cacheable|null
        +getExpiryTime(ResourceObject $ro, Cacheable|null $cacheable = null): int
    }
    MethodInterceptor <|.. HttpCacheInterceptor
    QueryRepository --> RepositoryLoggerInterface
    QueryRepository --> HeaderSetter
    QueryRepository --> ResourceStorageInterface
    QueryRepository --> Expiry
    HttpCacheInterceptor --> MethodInvocation
    QueryRepository --> ResourceObject
    HttpCacheInterceptor --> ResourceObject
    QueryRepository -.- Reader
Loading

File-Level Changes

Change Details Files
Core attributes migration
  • Removed doctrine/annotations and doctrine/cache from composer.json
  • Switched HttpCacheInterceptor to ReflectionClass->getAttributes and added NoHttpCache handling
  • Refactored QueryRepository: dropped Reader injection and replaced getClassAnnotation calls with getAttributes usage
  • Cleaned up tests by removing AnnotationReader instantiation
composer.json
src/HttpCacheInterceptor.php
src/QueryRepository.php
tests/ResourceRepositoryTest.php
User migration tooling and guide
  • Added rector-migrate.php for automated annotation-to-attribute conversion
  • Authored ANNOTATION_TO_ATTRIBUTE.md with step-by-step migration instructions, examples, and troubleshooting
rector-migrate.php
ANNOTATION_TO_ATTRIBUTE.md
Documentation updates
  • Bumped release date in CHANGELOG.md and documented removal of doctrine dependencies
  • Noted addition of migration tools and PHP 8 attributes migration in CHANGELOG.md
CHANGELOG.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 8, 2025

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

The PR migrates the codebase from Doctrine annotations to PHP 8 native attributes. Key changes include removing Doctrine annotation dependencies from composer.json, converting all docblock annotations to PHP 8 attributes, updating reflection-based code to read attributes instead of annotations, deprecating legacy cache interfaces, and providing migration tooling and documentation.

Changes

Cohort / File(s) Summary
Build & Platform Configuration
.scrutinizer.yml, vendor-bin/tools/composer.json
Updated Scrutinizer image from bionic to jammy; upgraded PHP 8.2 to 8.4; removed analysis security flags; added PHP 8.4.99 platform constraint.
Dependency Management
composer.json, composer-require-checker.json
Removed runtime dependencies doctrine/annotations and doctrine/cache; removed dev dependency ext-redis and koriym/attributes; added suggest section for ext-redis and ext-memcached; updated symbol-whitelist to replace ArrayCache with CacheProvider.
Deprecated Modules
src-deprecated/BcModule.php
Replaced DI bindings with runtime deprecation warning via trigger_error.
Annotation Class Definitions
src-annotation/CacheEngine.php, src-annotation/Cacheable.php, src-annotation/HttpCache.php, src-annotation/KnownTagTtl.php, src-annotation/Memcache.php, src-annotation/Purge.php, src-annotation/Redis.php, src-annotation/Refresh.php, src-annotation-deprecated/CacheVersion.php
Removed Doctrine-style docblock annotations and imports (NamedArgumentConstructor); retained PHP 8 attributes; CacheVersion no longer implements NamedArgumentConstructorAnnotation.
Core Library Logic
src/QueryRepository.php, src/HttpCacheInterceptor.php
Replaced Doctrine AnnotationReader with PHP 8 reflection API (getAttributes, newInstance); removed Reader constructor parameter and dependency.
Migration Tooling & Documentation
rector-migrate.php, ANNOTATION_TO_ATTRIBUTE.md
Added Rector configuration for automated annotation-to-attribute migration and comprehensive migration guide with before/after examples.
Test Infrastructure
tests-php8/AttributeTest.php, tests/ResourceRepositoryTest.php, tests/QueryRepositoryTest.php, tests/bootstrap.php
Removed Doctrine AnnotationReader and Reader dependencies; updated attribute retrieval logic; removed test fixtures for legacy annotation bindings.
Test Environment Requirements
tests-pecl-ext/StorageRedisDsnModuleTest.php, tests/DonutCommandRedisCacheTest.php, tests/StorageRedisDsnModuleMarshallerTest.php
Added @requires extension redis PHPDoc annotations to gate Redis-dependent tests.
Test Fixture Resource Classes
tests/Fake/fake-app/src/Resource/App/*.php, tests/Fake/fake-app/src/Resource/Page/*.php
Migrated all class and method-level docblock annotations (@Cacheable, @HttpCache, @Purge, @Refresh, etc.) to PHP 8 attributes across ~30 resource classes.
Demo Application
demo/AppModule.php, demo/Resource/App/User.php
Removed Doctrine cache imports and bindings; replaced docblock annotation with #[Cacheable] attribute.
Configuration & Documentation
CHANGELOG.md, .scrutinizer.yml, phpstan.neon
Updated release date; documented migration changes; removed PHPStan ignoreErrors suppression for specific test path.
Cleanup
vendor-bin/tool/composer.json
Removed empty manifest file.

Sequence Diagram

sequenceDiagram
    actor User
    participant OldFlow as Old: Doctrine Annotations
    participant NewFlow as New: PHP 8 Attributes
    participant Reflection as PHP Reflection
    
    User->>OldFlow: Request resource metadata
    OldFlow->>OldFlow: Initialize AnnotationReader
    OldFlow->>OldFlow: Parse docblock annotations
    OldFlow->>OldFlow: Instantiate Doctrine objects
    OldFlow-->>User: Return annotation instance
    
    User->>NewFlow: Request resource metadata
    NewFlow->>Reflection: getAttributes(className)
    Reflection->>Reflection: Query native attributes
    Reflection->>NewFlow: Return Attribute objects
    NewFlow->>Reflection: newInstance()
    NewFlow-->>User: Return attribute instance
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Areas requiring extra attention:

  • Core logic changes (src/QueryRepository.php, src/HttpCacheInterceptor.php): Verify correct attribute reading via PHP 8 reflection API, proper fallback handling (e.g., HttpCache/NoHttpCache precedence), and functional equivalence with Doctrine Reader behavior
  • Constructor signature change: QueryRepository no longer accepts Reader parameter; confirm all callers/dependents have been updated
  • Rector migration config (rector-migrate.php): Validate comprehensive coverage of all BEAR.RepositoryModule annotations and correct mapping syntax
  • Test environment gating: Ensure @requires extension redis annotations properly skip tests when Redis is unavailable
  • Deprecated module behavior: Confirm BcModule deprecation warning is emitted correctly and doesn't break existing users of the old interface

Possibly related PRs

Suggested reviewers

  • NaokiTsuchiya
  • jingu

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: migrating from doctrine/annotations to PHP 8 Attributes, which is the primary objective of this PR.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, covering core migration changes, user migration support, documentation updates, and the rationale for the change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (a1c16a2) to head (b9e1584).
⚠️ Report is 21 commits behind head on 1.x.

Additional details and impacted files
@@             Coverage Diff             @@
##                 1.x      #165   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity       239       243    +4     
===========================================
  Files             52        52           
  Lines            766       734   -32     
===========================================
- Hits             766       734   -32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `src/HttpCacheInterceptor.php:26-31` </location>
<code_context>
     public function invoke(MethodInvocation $invocation)
     {
-        $cacheControl = $invocation->getMethod()->getDeclaringClass()->getAnnotation(AbstractCacheControl::class);
+        $class = $invocation->getMethod()->getDeclaringClass();
+        $attributes = $class->getAttributes(HttpCache::class);
+        if (empty($attributes)) {
+            $attributes = $class->getAttributes(NoHttpCache::class);
+        }
+
+        $cacheControl = isset($attributes[0]) ? $attributes[0]->newInstance() : null;
         $ro = $invocation->proceed();
         assert($ro instanceof ResourceObject);
</code_context>

<issue_to_address>
**suggestion:** Consider handling multiple attribute instances for extensibility.

Only the first attribute is processed. To prevent misconfiguration, either enforce a single attribute or implement logic to handle multiple instances as needed.

```suggestion
        $attributes = $class->getAttributes(HttpCache::class);
        if (empty($attributes)) {
            $attributes = $class->getAttributes(NoHttpCache::class);
        }

        // Enforce a single attribute instance for cache control
        if (count($attributes) > 1) {
            throw new \LogicException(
                sprintf(
                    'Multiple cache control attributes found on class %s. Only one instance of HttpCache or NoHttpCache is allowed.',
                    $class->getName()
                )
            );
        }

        $cacheControl = isset($attributes[0]) ? $attributes[0]->newInstance() : null;
```
</issue_to_address>

### Comment 2
<location> `src/HttpCacheInterceptor.php:27-28` </location>
<code_context>
-        $cacheControl = $invocation->getMethod()->getDeclaringClass()->getAnnotation(AbstractCacheControl::class);
+        $class = $invocation->getMethod()->getDeclaringClass();
+        $attributes = $class->getAttributes(HttpCache::class);
+        if (empty($attributes)) {
+            $attributes = $class->getAttributes(NoHttpCache::class);
+        }
+
</code_context>

<issue_to_address>
**question:** The fallback to NoHttpCache may mask missing or misapplied attributes.

If neither attribute is present, $cacheControl will be null. Please confirm this is the intended behavior and that it won't cause silent failures or unexpected results.
</issue_to_address>

### Comment 3
<location> `src/QueryRepository.php:91-95` </location>
<code_context>

     private function getCacheableAnnotation(ResourceObject $ro): Cacheable|null
     {
-        return $this->reader->getClassAnnotation(new ReflectionClass($ro), Cacheable::class);
+        $attributes = (new ReflectionClass($ro))->getAttributes(Cacheable::class);
+
+        return isset($attributes[0]) ? $attributes[0]->newInstance() : null;
     }

</code_context>

<issue_to_address>
**suggestion:** Multiple Cacheable attributes are not handled and may cause silent misconfiguration.

Only the first Cacheable attribute is used if multiple are present. Please add validation to prevent multiple attributes or update documentation to clarify this behavior.

```suggestion
    private function getCacheableAnnotation(ResourceObject $ro): Cacheable|null
    {
        $attributes = (new ReflectionClass($ro))->getAttributes(Cacheable::class);

        if (count($attributes) > 1) {
            throw new \LogicException(
                sprintf(
                    'Multiple Cacheable attributes found on %s. Only one Cacheable attribute is allowed.',
                    get_class($ro)
                )
            );
        }

        return isset($attributes[0]) ? $attributes[0]->newInstance() : null;
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/HttpCacheInterceptor.php
Comment on lines +27 to +28
if (empty($attributes)) {
$attributes = $class->getAttributes(NoHttpCache::class);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: The fallback to NoHttpCache may mask missing or misapplied attributes.

If neither attribute is present, $cacheControl will be null. Please confirm this is the intended behavior and that it won't cause silent failures or unexpected results.

Comment thread src/QueryRepository.php
koriym and others added 3 commits November 9, 2025 11:55
Remove all Doctrine annotation-related imports and docblock annotations from attribute classes in src-annotation/:

- Remove `use Doctrine\Common\Annotations\Annotation\NamedArgumentConstructor`
- Remove `@Annotation`, `@Target`, `@NamedArgumentConstructor` docblock annotations
- Keep only PHP 8 Attribute declarations and `@see` documentation

Affected files:
- Cacheable.php
- HttpCache.php
- Refresh.php
- Purge.php
- KnownTagTtl.php
- Redis.php
- CacheEngine.php
- Memcache.php

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
…HP Reflection

Complete migration from annotation readers to native PHP 8 Reflection API:

- Remove koriym/attributes from composer.json dev dependencies
- Update AttributeTest to use native getAttributes() instead of annotation readers
- Remove AttributeReader and ServiceLocator setup from tests/bootstrap.php
- Remove all docblock annotations from test fixture files
- Clean up empty docblocks left after annotation removal

Test fixtures updated (32 files):
- Remove @Cacheable, @refresh, @purge, @HttpCache docblock annotations
- Keep only PHP 8 #[Attribute] syntax
- All tests pass (102 tests, 198 assertions)

This completes the transition to native PHP 8 Attributes, eliminating all
dependencies on the abandoned doctrine/annotations package and its wrappers.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
… dependencies

Clean up all remaining references to abandoned Doctrine packages:

- Update composer-require-checker.json whitelist to use CacheProvider instead of ArrayCache
- Remove Doctrine\Common\Cache bindings from demo/AppModule.php
- Remove NamedArgumentConstructorAnnotation from CacheVersion (deprecated)
- Simplify BcModule to only emit deprecation warning, remove all Doctrine bindings
- Remove unused AnnotationReader imports from tests
- Remove Doctrine CacheProvider test case from ResourceRepositoryTest

All code now uses native PHP 8 Attributes and Symfony Cache components only.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Remove ext-redis from require-dev and add @requires extension redis to test classes that need it:

- Remove ext-redis from composer.json require-dev
- Add suggest section for ext-redis and ext-memcached
- Add @requires extension redis to DonutCommandRedisCacheTest
- Add @requires extension redis to StorageRedisDsnModuleMarshallerTest
- Add @requires extension redis to StorageRedisDsnModuleTest

This allows developers to run composer install without Redis extension installed.
Tests requiring Redis will be automatically skipped by PHPUnit when the extension is not available.
Composer will suggest installing the extensions when they are needed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
koriym and others added 2 commits November 9, 2025 15:57
Remove ignoreErrors pattern for CacheProvider test code that was deleted in commit 7e7a055.
The 'but return statement is missing' error no longer occurs since the Doctrine CacheProvider
test case was removed from ResourceRepositoryTest.php.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Remove leftover Doctrine annotation references from demo application:

- Remove commented-out CacheProvider binding from demo/AppModule.php
- Remove @Cacheable docblock annotation from demo/Resource/App/User.php (keep #[Cacheable] attribute)

Demo application now uses only native PHP 8 Attributes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@koriym
Copy link
Copy Markdown
Member Author

koriym commented Nov 9, 2025

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 9, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
src-deprecated/BcModule.php (2)

8-8: Optional: The use function import is not strictly necessary.

Since trigger_error is a built-in PHP function, the import is optional. However, it can improve IDE support and makes the dependency explicit, so this is acceptable as-is.


23-26: Improve the deprecation message to provide clearer migration guidance.

The review comment's suggestion is valid. While BcModule is not referenced anywhere in the codebase and removal is safe, the deprecation message should guide users on the migration path since the old bindings this module provided are now available through other modules (e.g., CacheableModule for HttpCacheInterface).

Apply this improvement:

-        trigger_error('BEAR\QueryRepository\BcModule is deprecated.', E_USER_DEPRECATED);
+        trigger_error('BEAR\QueryRepository\BcModule is deprecated and no longer provides any bindings. Remove it from your module installation.', E_USER_DEPRECATED);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1c16a2 and 2be656d.

⛔ Files ignored due to path filters (2)
  • vendor-bin/tool/composer.lock is excluded by !**/*.lock
  • vendor-bin/tools/composer.lock is excluded by !**/*.lock
📒 Files selected for processing (62)
  • .scrutinizer.yml (1 hunks)
  • ANNOTATION_TO_ATTRIBUTE.md (1 hunks)
  • CHANGELOG.md (2 hunks)
  • composer-require-checker.json (1 hunks)
  • composer.json (1 hunks)
  • demo/AppModule.php (0 hunks)
  • demo/Resource/App/User.php (0 hunks)
  • phpstan.neon (0 hunks)
  • rector-migrate.php (1 hunks)
  • src-annotation-deprecated/CacheVersion.php (1 hunks)
  • src-annotation/CacheEngine.php (0 hunks)
  • src-annotation/Cacheable.php (0 hunks)
  • src-annotation/HttpCache.php (0 hunks)
  • src-annotation/KnownTagTtl.php (0 hunks)
  • src-annotation/Memcache.php (0 hunks)
  • src-annotation/Purge.php (1 hunks)
  • src-annotation/Redis.php (0 hunks)
  • src-annotation/Refresh.php (1 hunks)
  • src-deprecated/BcModule.php (2 hunks)
  • src/HttpCacheInterceptor.php (2 hunks)
  • src/QueryRepository.php (1 hunks)
  • tests-pecl-ext/StorageRedisDsnModuleTest.php (1 hunks)
  • tests-php8/AttributeTest.php (1 hunks)
  • tests/DonutCommandRedisCacheTest.php (1 hunks)
  • tests/Fake/fake-app/src/Resource/App/Code.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/ControlExpiry.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/ControlExpiryError.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/ControlNone.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/ControlPublic.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/Dep/LevelOne.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/Entry.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/Etag.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/HttpCacheControl.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/HttpCacheControlOverrideMaxAge.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/HttpCacheControlWithCacheable.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/Invalid.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/NullView.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/SometimesSameResponse.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/TypedParam.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/Unmatch.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/User.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/User/Profile.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/Value.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/App/View.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/Page/Dep/LevelOne.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/Page/Dep/LevelThree.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/Page/Dep/LevelTwo.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/Page/EmbVal.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/Page/EmbView.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/Page/Html/BlogPosting.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/Page/Html/BlogPostingCache.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/Page/Html/BlogPostingCacheControl.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/Page/Html/Comment.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/Page/Html/Like.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/Page/Html/PageSurrogateKey.php (0 hunks)
  • tests/Fake/fake-app/src/Resource/Page/Index.php (0 hunks)
  • tests/QueryRepositoryTest.php (0 hunks)
  • tests/ResourceRepositoryTest.php (0 hunks)
  • tests/StorageRedisDsnModuleMarshallerTest.php (1 hunks)
  • tests/bootstrap.php (0 hunks)
  • vendor-bin/tool/composer.json (0 hunks)
  • vendor-bin/tools/composer.json (1 hunks)
💤 Files with no reviewable changes (45)
  • phpstan.neon
  • tests/Fake/fake-app/src/Resource/Page/Html/Comment.php
  • vendor-bin/tool/composer.json
  • src-annotation/CacheEngine.php
  • tests/QueryRepositoryTest.php
  • tests/Fake/fake-app/src/Resource/Page/Index.php
  • tests/Fake/fake-app/src/Resource/App/User/Profile.php
  • src-annotation/Redis.php
  • tests/Fake/fake-app/src/Resource/Page/Html/BlogPostingCacheControl.php
  • tests/Fake/fake-app/src/Resource/App/View.php
  • demo/Resource/App/User.php
  • tests/Fake/fake-app/src/Resource/App/HttpCacheControl.php
  • tests/Fake/fake-app/src/Resource/App/ControlPublic.php
  • tests/Fake/fake-app/src/Resource/App/ControlExpiry.php
  • tests/Fake/fake-app/src/Resource/Page/Html/Like.php
  • tests/Fake/fake-app/src/Resource/App/HttpCacheControlOverrideMaxAge.php
  • tests/Fake/fake-app/src/Resource/App/ControlNone.php
  • tests/Fake/fake-app/src/Resource/App/TypedParam.php
  • src-annotation/KnownTagTtl.php
  • tests/Fake/fake-app/src/Resource/Page/EmbVal.php
  • tests/Fake/fake-app/src/Resource/App/Dep/LevelOne.php
  • tests/Fake/fake-app/src/Resource/Page/Html/BlogPosting.php
  • tests/Fake/fake-app/src/Resource/Page/EmbView.php
  • tests/Fake/fake-app/src/Resource/App/Entry.php
  • tests/Fake/fake-app/src/Resource/App/ControlExpiryError.php
  • tests/ResourceRepositoryTest.php
  • tests/Fake/fake-app/src/Resource/App/User.php
  • tests/Fake/fake-app/src/Resource/App/NullView.php
  • tests/Fake/fake-app/src/Resource/Page/Dep/LevelThree.php
  • tests/Fake/fake-app/src/Resource/App/Code.php
  • tests/Fake/fake-app/src/Resource/App/HttpCacheControlWithCacheable.php
  • tests/Fake/fake-app/src/Resource/App/SometimesSameResponse.php
  • tests/Fake/fake-app/src/Resource/Page/Dep/LevelTwo.php
  • tests/Fake/fake-app/src/Resource/App/Unmatch.php
  • tests/bootstrap.php
  • tests/Fake/fake-app/src/Resource/App/Value.php
  • tests/Fake/fake-app/src/Resource/App/Invalid.php
  • src-annotation/Memcache.php
  • tests/Fake/fake-app/src/Resource/App/Etag.php
  • tests/Fake/fake-app/src/Resource/Page/Html/PageSurrogateKey.php
  • tests/Fake/fake-app/src/Resource/Page/Dep/LevelOne.php
  • tests/Fake/fake-app/src/Resource/Page/Html/BlogPostingCache.php
  • src-annotation/HttpCache.php
  • src-annotation/Cacheable.php
  • demo/AppModule.php
🧰 Additional context used
📓 Path-based instructions (6)
**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

Adhere to PSR-12 coding standards using PHP_CodeSniffer for all PHP source files

Files:

  • src/QueryRepository.php
  • src-deprecated/BcModule.php
  • src-annotation-deprecated/CacheVersion.php
  • tests/StorageRedisDsnModuleMarshallerTest.php
  • src-annotation/Purge.php
  • src/HttpCacheInterceptor.php
  • rector-migrate.php
  • tests-pecl-ext/StorageRedisDsnModuleTest.php
  • tests-php8/AttributeTest.php
  • tests/DonutCommandRedisCacheTest.php
  • src-annotation/Refresh.php
{src,tests}/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

{src,tests}/**/*.php: Ensure code passes PHPStan at max level as configured by phpstan.neon
Ensure code passes Psalm at error level 1 as configured by psalm.xml

Files:

  • src/QueryRepository.php
  • tests/StorageRedisDsnModuleMarshallerTest.php
  • src/HttpCacheInterceptor.php
  • tests/DonutCommandRedisCacheTest.php
tests/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

Place unit tests under tests/

Files:

  • tests/StorageRedisDsnModuleMarshallerTest.php
  • tests/DonutCommandRedisCacheTest.php
src-annotation/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

Define and maintain cache-related attributes #[Cacheable], #[HttpCache], #[Refresh], #[Purge] in src-annotation/

Files:

  • src-annotation/Purge.php
  • src-annotation/Refresh.php
tests-pecl-ext/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

Place PECL extension-related tests under tests-pecl-ext/

Files:

  • tests-pecl-ext/StorageRedisDsnModuleTest.php
tests-php8/**/*.php

📄 CodeRabbit inference engine (CLAUDE.md)

Place PHP 8-specific tests under tests-php8/

Files:

  • tests-php8/AttributeTest.php
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: bearsunday/BEAR.QueryRepository PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T05:23:58.622Z
Learning: Applies to src-annotation/**/*.php : Define and maintain cache-related attributes #[Cacheable], #[HttpCache], #[Refresh], #[Purge] in src-annotation/
📚 Learning: 2025-01-09T11:57:54.020Z
Learnt from: koriym
Repo: bearsunday/BEAR.QueryRepository PR: 159
File: composer.json:26-26
Timestamp: 2025-01-09T11:57:54.020Z
Learning: In BEAR.QueryRepository, the `bear/fastly-module` package is maintained as a development dependency in `require-dev` as it's not required for core functionality.

Applied to files:

  • composer.json
  • src-deprecated/BcModule.php
📚 Learning: 2025-10-21T05:23:58.622Z
Learnt from: CR
Repo: bearsunday/BEAR.QueryRepository PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-21T05:23:58.622Z
Learning: Applies to src-annotation/**/*.php : Define and maintain cache-related attributes #[Cacheable], #[HttpCache], #[Refresh], #[Purge] in src-annotation/

Applied to files:

  • src/QueryRepository.php
  • src-annotation-deprecated/CacheVersion.php
  • src-annotation/Purge.php
  • src/HttpCacheInterceptor.php
  • ANNOTATION_TO_ATTRIBUTE.md
  • rector-migrate.php
  • tests-php8/AttributeTest.php
  • tests/DonutCommandRedisCacheTest.php
  • src-annotation/Refresh.php
📚 Learning: 2024-11-01T14:52:06.297Z
Learnt from: koriym
Repo: bearsunday/BEAR.QueryRepository PR: 144
File: src-deprecated/ResourceStorageCacheableTrait.php:24-24
Timestamp: 2024-11-01T14:52:06.297Z
Learning: When deprecating the `ResourceStorageCacheableTrait` in `src-deprecated/ResourceStorageCacheableTrait.php`, it is acceptable to only add the `deprecated` annotation without adding a runtime deprecation notice using `trigger_deprecation()`.

Applied to files:

  • src-annotation-deprecated/CacheVersion.php
📚 Learning: 2024-11-01T15:45:27.018Z
Learnt from: koriym
Repo: bearsunday/BEAR.QueryRepository PR: 144
File: src/StorageRedisDsnModule.php:53-68
Timestamp: 2024-11-01T15:45:27.018Z
Learning: In `src/StorageRedisDsnModule.php`, lines 58 and 59 may both bind the Redis provider, and this can be necessary in certain cases.

Applied to files:

  • tests/StorageRedisDsnModuleMarshallerTest.php
  • tests-pecl-ext/StorageRedisDsnModuleTest.php
📚 Learning: 2024-11-24T01:43:38.518Z
Learnt from: koriym
Repo: bearsunday/BEAR.QueryRepository PR: 148
File: .github/workflows/continuous-integration.yml:46-51
Timestamp: 2024-11-24T01:43:38.518Z
Learning: PHP 8.4 has been officially released and can be included in the CI workflows without marking it as experimental.

Applied to files:

  • CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Sourcery review
  • GitHub Check: PHPUnit (macos-latest - PHP 8.5 - lowest)
🔇 Additional comments (24)
src-deprecated/BcModule.php (1)

15-15: LGTM: Grammar improvement.

The addition of the article "a" improves the documentation readability.

vendor-bin/tools/composer.json (1)

15-17: LGTM! Platform constraint aligns with PHP 8.4 upgrade.

The platform configuration correctly sets PHP 8.4.99 for forward compatibility testing of development tools, which aligns with the broader PHP 8.4 upgrade across the CI environment.

tests-pecl-ext/StorageRedisDsnModuleTest.php (1)

23-25: LGTM! Proper PHPUnit extension requirement annotation.

The @requires extension redis annotation correctly ensures this test only runs when the Redis extension is available, which aligns with the removal of ext-redis from require-dev and its move to the suggest section in composer.json.

composer-require-checker.json (1)

7-9: LGTM! Updated Doctrine cache symbol whitelist.

The change from ArrayCache to the more generic CacheProvider base class in the symbol whitelist is appropriate and aligns with the broader migration away from doctrine/cache while maintaining backward compatibility for code that may reference these cache implementations.

rector-migrate.php (1)

1-37: LGTM! Comprehensive Rector migration configuration.

The configuration properly sets up automated migration from Doctrine annotations to PHP 8 attributes. The annotation mappings are complete and the usage examples in the docblock are helpful.

Minor observation: The configuration includes RefreshCache (line 34) and Commands (line 35) annotations beyond the six documented in the PR objectives. This appears intentional for comprehensive coverage.

.scrutinizer.yml (3)

2-2: LGTM! Ubuntu base image upgrade.

Upgrading from default-bionic (Ubuntu 18.04) to default-jammy (Ubuntu 22.04) is appropriate for PHP 8.4 support and ensures the build environment is on a supported Ubuntu LTS release.


5-5: LGTM! PHP version upgraded to 8.4.

The PHP version upgrade from 8.2 to 8.4 aligns with the PR's migration to PHP 8 attributes and the broader CI/runtime updates.


8-12: Security analysis coverage gap confirmed.

The verification shows that no active security scanning tools (Snyk, SonarQube, Psalm, PHPStan) are configured in the GitHub CI pipeline. The codebase relies on Dependabot for dependency updates, but this does not replace code security analysis.

The removal of --enable-security-analysis from php-scrutinizer-run appears to create a gap in your security coverage unless:

  • Scrutinizer performs security analysis by default without the flag, or
  • Security analysis is handled through an alternative mechanism not visible in the CI configuration

Recommend either restoring the flag or documenting the security analysis strategy elsewhere in your pipeline.

src-annotation/Purge.php (1)

9-13: LGTM! Proper migration to PHP 8 attribute.

The migration from Doctrine annotations to PHP 8's native #[Attribute] is correctly implemented:

  • Attribute::TARGET_METHOD properly restricts usage to methods
  • Attribute::IS_REPEATABLE allows multiple #[Purge] attributes on the same method, which is appropriate for cache purging operations

This aligns with the coding guideline for defining cache-related attributes in src-annotation/.

Based on learnings.

composer.json (1)

39-42: LGTM! Proper dependency migration to optional suggestions.

The new suggest section appropriately documents optional cache backends:

  • Moving ext-redis from require-dev to suggest correctly reflects that it's an optional backend choice
  • Adding ext-memcached provides users clear guidance on cache storage options
  • This aligns with the removal of doctrine/cache and the migration to native PHP 8 attributes

The descriptive messages clearly explain why each extension is needed.

CHANGELOG.md (3)

13-14: LGTM! Migration tooling properly documented.

The changelog correctly documents the addition of:

  • rector-migrate.php for automated migration
  • ANNOTATION_TO_ATTRIBUTE.md for comprehensive migration instructions

This provides clear visibility to users about the available migration tools.


30-30: LGTM! Major migration clearly documented.

The PHP 8 Attributes Migration is properly documented in the Changed section with clear indication of the removed dependencies (doctrine/annotations and doctrine/cache) and the migration to native PHP 8 attributes.


10-10: Date verification confirmed—no changes needed.

The release date 2025-11-09 is correct. Today is Sunday, November 9, 2025, and the CHANGELOG entry accurately reflects today's date.

src-annotation-deprecated/CacheVersion.php (1)

16-16: LGTM! Interface removal aligns with attribute migration.

Removing the NamedArgumentConstructorAnnotation interface is correct, as it was Doctrine-specific and no longer needed with PHP 8 attributes.

ANNOTATION_TO_ATTRIBUTE.md (1)

1-319: Excellent migration guide!

This comprehensive documentation provides clear rationale, step-by-step instructions, accurate before/after examples, and helpful troubleshooting guidance. The Rector-based approach with provided configuration will make user migration smooth and automated.

src-annotation/Refresh.php (1)

10-10: LGTM! Docblock cleanup completed.

Replacing Doctrine annotation metadata with a simple @see reference is correct and maintains useful documentation without the deprecated annotation infrastructure.

tests/DonutCommandRedisCacheTest.php (1)

17-17: Good practice: gating Redis-dependent test.

Adding @requires extension redis ensures the test is skipped gracefully when the Redis extension is unavailable, preventing false failures.

tests/StorageRedisDsnModuleMarshallerTest.php (1)

12-12: Good practice: gating Redis-dependent test.

Adding @requires extension redis ensures the test is skipped gracefully when the Redis extension is unavailable.

src/HttpCacheInterceptor.php (2)

8-9: LGTM! Import statements added for attribute classes.

Adding use statements for HttpCache and NoHttpCache is necessary for the attribute-based implementation.


25-31: LGTM! Successful migration to PHP 8 attributes.

The attribute retrieval logic is correct:

  • Uses native getAttributes() API instead of Doctrine Reader
  • Checks HttpCache first, falls back to NoHttpCache
  • Takes the first attribute (PHP enforces single instance for non-repeatable attributes)
  • Null handling is correct (checked on line 34)
src/QueryRepository.php (3)

22-28: LGTM! Reader dependency successfully removed.

The constructor no longer requires the Doctrine Reader, completing the migration to PHP 8 attributes. This eliminates the dependency on the abandoned doctrine/annotations package.


86-88: LGTM! HttpCache annotation retrieval migrated to attributes.

The method correctly uses the native PHP 8 attributes API (getAttributes() and newInstance()) instead of the Doctrine Reader.


93-95: LGTM! Cacheable annotation retrieval migrated to attributes.

The method correctly uses the native PHP 8 attributes API. Since Cacheable is non-repeatable, selecting the first attribute is safe.

tests-php8/AttributeTest.php (1)

17-42: LGTM! Test successfully migrated to PHP 8 attributes.

The test now directly uses the native PHP 8 attributes API:

  • Removed Reader dependency and data provider
  • Uses getAttributes() and newInstance() throughout
  • Validates both class-level (Cacheable, NoHttpCache) and method-level (Purge, Refresh) attributes
  • Clean, straightforward validation of the attribute migration

…nstallation

Remove hardcoded paths from rector-migrate.php to support vendor installation.
Users now specify paths via command-line arguments, following Ray.AuraSqlModule pattern.

- Remove `withPaths()` from rector-migrate.php
- Add usage examples for single and multiple directory processing
- Simplify migration guide steps (remove version update step)
- Add Ray.Di annotation migration note
- Update test command to vendor/bin/phpunit
- Add missing annotations to supported list
Temporarily use the dev-fix-legacy-annotations branch to fix AnnotationException
caused by legacy doctrine annotation comments in provider classes.

Related: ray-di/Ray.PsrCacheModule#33
Remove @annotation, @target, and @qualifier docblock tags from attribute classes.
These legacy doctrine annotation markers are no longer needed with PHP 8 attributes
and could cause conflicts with packages still using DualReader.
Remove remaining doctrine-style docblock annotations from test fixture files:
- Remove @DonutCache, @NoHttpCache, @embed annotations from docblocks
- Keep only PHP 8 Attributes (#[...])
- Convert @FakeAnnotation docblock to inline comment for test coverage documentation

This fixes CI errors where doctrine/annotations parser was failing on classes
without @annotation marker in their docblocks.
- Update minimum PHP version from 8.1 to 8.2
- Remove unnecessary comment from User.php test fixture
@koriym koriym merged commit 5ef5a69 into 1.x Nov 10, 2025
38 checks passed
@koriym koriym deleted the php82 branch November 10, 2025 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant